Conversation
ianthomas23
left a comment
ianthomas23
left a comment
There was a problem hiding this comment.
The git tag -l v* definitely won't work in the wasm build. so I'll need to support that soon.
src/subcommand/tag_subcommand.cpp
Outdated
There was a problem hiding this comment.
If pos + 1 < message.length() is true then pos < message.length() must also be so it is not necessary to check both.
| } | ||
|
|
||
| // Tag listing: Print individual message lines | ||
| void print_list_lines(const std::string& message, int num_lines) |
There was a problem hiding this comment.
This function looks quite complicated for a relatively simple task. Maybe a better approach would be to first split the string at newline characters into a vector of strings and walk that vector. There is already a split-string-at-newlines function at
git2cpp/src/utils/terminal_pager.cpp
Line 199 in 4bd9b8e
which could be pulled out of that file and reused.
But that could be in a later separate PR.
src/subcommand/tag_subcommand.cpp
Outdated
There was a problem hiding this comment.
Alternatively we could avoid the loop variable here by using a range-based for loop like this instead:
for (const auto& tag_name: tag_names)
{
each_tag(repo, tag_name, m_num_lines);
}
src/wrapper/repository_wrapper.cpp
Outdated
There was a problem hiding this comment.
Could use a range-based for loop here too.
src/subcommand/tag_subcommand.cpp
Outdated
There was a problem hiding this comment.
This method has a lot in common with create_lightweight_tag, maybe it would be worth extracting the common parts in small functions and have the create_XXX_tag be simple "drivers".
src/subcommand/tag_subcommand.cpp
Outdated
There was a problem hiding this comment.
This condition is already tested before calling create_tag, so this check can be removed.
3 participants
Fix #64
Add
tagsubcommand.